Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Client gametest threading tweaks #4304

Merged

Conversation

Earthcomputer
Copy link
Contributor

@Earthcomputer Earthcomputer commented Dec 16, 2024

  • Fixed cases where the game didn't close properly when a test is thrown or an exception is thrown elsewhere.
  • Mutate the stack traces of exceptions thrown in tasks to make them easier to track.

You can review the commits one at a time to make it easier to review.

Copy link
Contributor

@Alexander01998 Alexander01998 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tested this by throwing a RuntimeException on each of the threads just after the in_game_overworld screenshot. The test thread and client thread worked fine, but the server thread did not, producing this stack trace:

[12:29:54] [Test thread/ERROR] (fabric-client-gametest-api-v1) Client gametests failed with an exception
java.lang.NullPointerException: stackTrace[9]
        at java.base/java.lang.Throwable.setStackTrace(Throwable.java:903) ~[?:?]
        at knot/net.fabricmc.fabric.impl.client.gametest.ThreadingImpl.joinAsyncStackTrace(ThreadingImpl.java:261) ~[fabric-client-gametest-api-v1-1.0.0+3dfdebf604-dev.jar:?]
        at knot/net.fabricmc.fabric.impl.client.gametest.ThreadingImpl.runTaskOnOtherThread(ThreadingImpl.java:195) ~[fabric-client-gametest-api-v1-1.0.0+3dfdebf604-dev.jar:?]
        at knot/net.fabricmc.fabric.impl.client.gametest.ThreadingImpl.runOnServer(ThreadingImpl.java:179) ~[fabric-client-gametest-api-v1-1.0.0+3dfdebf604-dev.jar:?]
        at knot/net.fabricmc.fabric.test.client.gametest.ClientGameTestTest.runTest(ClientGameTestTest.java:84) ~[testmodClient/:?]
        at knot/net.fabricmc.fabric.impl.client.gametest.FabricClientGameTestRunner.lambda$start$0(FabricClientGameTestRunner.java:43) ~[fabric-client-gametest-api-v1-1.0.0+3dfdebf604-dev.jar:?]
        at knot/net.fabricmc.fabric.impl.client.gametest.ThreadingImpl.lambda$runTestThread$1(ThreadingImpl.java:129) ~[fabric-client-gametest-api-v1-1.0.0+3dfdebf604-dev.jar:?]
        at java.base/java.lang.Thread.run(Thread.java:1583) [?:?]
[12:29:54] [Render thread/ERROR] (FabricLoader) Minecraft has crashed!
net.fabricmc.loader.impl.FormattedException: java.lang.NullPointerException: stackTrace[9]
        at net.fabricmc.loader.impl.FormattedException.ofLocalized(FormattedException.java:63) ~[fabric-loader-0.16.9.jar:?]
        at net.fabricmc.loader.impl.game.minecraft.MinecraftGameProvider.launch(MinecraftGameProvider.java:482) ~[fabric-loader-0.16.9.jar:?]
        at net.fabricmc.loader.impl.launch.knot.Knot.launch(Knot.java:74) [fabric-loader-0.16.9.jar:?]
        at net.fabricmc.loader.impl.launch.knot.KnotClient.main(KnotClient.java:23) [fabric-loader-0.16.9.jar:?]
        at net.fabricmc.devlaunchinjector.Main.main(Main.java:86) [dev-launch-injector-0.2.1+build.8.jar:?]
Caused by: java.lang.NullPointerException: stackTrace[9]
        at java.base/java.lang.Throwable.setStackTrace(Throwable.java:903) ~[?:?]
        at knot/net.fabricmc.fabric.impl.client.gametest.ThreadingImpl.joinAsyncStackTrace(ThreadingImpl.java:261) ~[fabric-client-gametest-api-v1-1.0.0+3dfdebf604-dev.jar:?]
        at knot/net.fabricmc.fabric.impl.client.gametest.ThreadingImpl.runTaskOnOtherThread(ThreadingImpl.java:195) ~[fabric-client-gametest-api-v1-1.0.0+3dfdebf604-dev.jar:?]
        at knot/net.fabricmc.fabric.impl.client.gametest.ThreadingImpl.runOnServer(ThreadingImpl.java:179) ~[fabric-client-gametest-api-v1-1.0.0+3dfdebf604-dev.jar:?]
        at knot/net.fabricmc.fabric.test.client.gametest.ClientGameTestTest.runTest(ClientGameTestTest.java:84) ~[testmodClient/:?]
        at knot/net.fabricmc.fabric.impl.client.gametest.FabricClientGameTestRunner.lambda$start$0(FabricClientGameTestRunner.java:43) ~[fabric-client-gametest-api-v1-1.0.0+3dfdebf604-dev.jar:?]
        at knot/net.fabricmc.fabric.impl.client.gametest.ThreadingImpl.lambda$runTestThread$1(ThreadingImpl.java:129) ~[fabric-client-gametest-api-v1-1.0.0+3dfdebf604-dev.jar:?]
        at java.base/java.lang.Thread.run(Thread.java:1583) ~[?:?]

Edit: To clarify, the correct line number ClientGameTestTest.java:84 is still in there, but the NullPointerException when joining the stack traces doesn't seem right.

Throwing the exception after the server_in_game screenshot instead produced a very similar result, so this issue does not seem to be specific to the integrated singleplayer server.

Code to reproduce:

Singleplayer server:

		{
			enableDebugHud(context);
			waitForWorldTicks(context, 200);
			context.takeScreenshot("in_game_overworld", 0);
+			ThreadingImpl.runOnServer(() -> {
+				throw new RuntimeException("test");
+			});
		}

Dedicated server:

			waitForWorldTicks(context, 20);
			context.takeScreenshot("server_in_game", 0);
+			ThreadingImpl.runOnServer(() -> {
+				throw new RuntimeException("test");
+			});

@Earthcomputer
Copy link
Contributor Author

but the server thread did not, producing this stack trace

try now

@Alexander01998
Copy link
Contributor

It works now.

[14:12:45] [Test thread/ERROR] (fabric-client-gametest-api-v1) Client gametests failed with an exception
 java.lang.RuntimeException: test
        at knot/net.fabricmc.fabric.test.client.gametest.ClientGameTestTest.lambda$runTest$0(ClientGameTestTest.java:85) ~[testmodClient/:?]
        at knot/net.fabricmc.fabric.impl.client.gametest.ThreadingImpl.runTaskOnThisThread(ThreadingImpl.java:203) ~[fabric-client-gametest-api-v1-1.0.0+db066d5704-dev.jar:?]
        at Async Stack Trace..(Unknown Source) ~[?:?]
        at knot/net.fabricmc.fabric.impl.client.gametest.ThreadingImpl.runTaskOnOtherThread(ThreadingImpl.java:195) ~[fabric-client-gametest-api-v1-1.0.0+db066d5704-dev.jar:?]
        at knot/net.fabricmc.fabric.impl.client.gametest.ThreadingImpl.runOnServer(ThreadingImpl.java:179) ~[fabric-client-gametest-api-v1-1.0.0+db066d5704-dev.jar:?]
        at knot/net.fabricmc.fabric.test.client.gametest.ClientGameTestTest.runTest(ClientGameTestTest.java:84) ~[testmodClient/:?]
        at knot/net.fabricmc.fabric.impl.client.gametest.FabricClientGameTestRunner.lambda$start$0(FabricClientGameTestRunner.java:43) ~[fabric-client-gametest-api-v1-1.0.0+db066d5704-dev.jar:?]
        at knot/net.fabricmc.fabric.impl.client.gametest.ThreadingImpl.lambda$runTestThread$1(ThreadingImpl.java:129) ~[fabric-client-gametest-api-v1-1.0.0+db066d5704-dev.jar:?]
        at java.base/java.lang.Thread.run(Thread.java:1583) [?:?]
[14:12:45] [Server thread/INFO] (ServerChunkLifecycleTests) Loaded 3 freshly generated chunks in minecraft:overworld during tick #223
[14:12:45] [Render thread/ERROR] (FabricLoader) Minecraft has crashed!
 net.fabricmc.loader.impl.FormattedException: java.lang.RuntimeException: test
        at net.fabricmc.loader.impl.FormattedException.ofLocalized(FormattedException.java:63) ~[fabric-loader-0.16.9.jar:?]
        at net.fabricmc.loader.impl.game.minecraft.MinecraftGameProvider.launch(MinecraftGameProvider.java:482) ~[fabric-loader-0.16.9.jar:?]
        at net.fabricmc.loader.impl.launch.knot.Knot.launch(Knot.java:74) [fabric-loader-0.16.9.jar:?]
        at net.fabricmc.loader.impl.launch.knot.KnotClient.main(KnotClient.java:23) [fabric-loader-0.16.9.jar:?]
        at net.fabricmc.devlaunchinjector.Main.main(Main.java:86) [dev-launch-injector-0.2.1+build.8.jar:?]
Caused by: java.lang.RuntimeException: test
        at knot/net.fabricmc.fabric.test.client.gametest.ClientGameTestTest.lambda$runTest$0(ClientGameTestTest.java:85) ~[testmodClient/:?]
        at knot/net.fabricmc.fabric.impl.client.gametest.ThreadingImpl.runTaskOnThisThread(ThreadingImpl.java:203) ~[fabric-client-gametest-api-v1-1.0.0+db066d5704-dev.jar:?]
        at Async Stack Trace..(Unknown Source) ~[?:?]
        at knot/net.fabricmc.fabric.impl.client.gametest.ThreadingImpl.runTaskOnOtherThread(ThreadingImpl.java:195) ~[fabric-client-gametest-api-v1-1.0.0+db066d5704-dev.jar:?]
        at knot/net.fabricmc.fabric.impl.client.gametest.ThreadingImpl.runOnServer(ThreadingImpl.java:179) ~[fabric-client-gametest-api-v1-1.0.0+db066d5704-dev.jar:?]
        at knot/net.fabricmc.fabric.test.client.gametest.ClientGameTestTest.runTest(ClientGameTestTest.java:84) ~[testmodClient/:?]
        at knot/net.fabricmc.fabric.impl.client.gametest.FabricClientGameTestRunner.lambda$start$0(FabricClientGameTestRunner.java:43) ~[fabric-client-gametest-api-v1-1.0.0+db066d5704-dev.jar:?]
        at knot/net.fabricmc.fabric.impl.client.gametest.ThreadingImpl.lambda$runTestThread$1(ThreadingImpl.java:129) ~[fabric-client-gametest-api-v1-1.0.0+db066d5704-dev.jar:?]
        at java.base/java.lang.Thread.run(Thread.java:1583) ~[?:?]

Copy link
Member

@modmuss50 modmuss50 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A little bit beyond me, but I trust its working. Makes sense at a high level 👍

@modmuss50 modmuss50 added bug Something isn't working merge me please Pull requests that are ready to merge labels Dec 17, 2024
Copy link
Contributor

@Alexander01998 Alexander01998 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried a few other ways to break it, like adding a custom shutdown hook to the testmod and throwing exceptions in other places where they might plausibly happen. Didn't find any issues.

@modmuss50 modmuss50 merged commit b3e1e3d into FabricMC:1.21.4 Dec 18, 2024
4 checks passed
@Earthcomputer Earthcomputer deleted the client-gametest-threading-tweaks branch December 18, 2024 22:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working merge me please Pull requests that are ready to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants